Skip to content

Fix for DelayedVector that are not geojson format.#522

Draft
EmileSonneveld wants to merge 4 commits into
masterfrom
to_save_result_fix
Draft

Fix for DelayedVector that are not geojson format.#522
EmileSonneveld wants to merge 4 commits into
masterfrom
to_save_result_fix

Conversation

@EmileSonneveld

@EmileSonneveld EmileSonneveld commented Jun 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Found while fixing typings with mypy
Related Open-EO/openeo-geopyspark-driver#793

@Open-EO Open-EO deleted a comment from chatgpt-codex-connector Bot Jun 29, 2026
@EmileSonneveld

EmileSonneveld commented Jun 29, 2026

Copy link
Copy Markdown
Member Author

@copilot review without making changes

Copilot AI commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

@copilot review without making changes

I found one substantive issue: DelayedVector.as_geodataframe() currently builds a GeoDataFrame from self.geometries only, so feature properties are dropped. As a result, to_save_result(DelayedVector(...), format="parquet") writes only the geometry column, while id and pop from FeatureCollection02.json are lost. That matches the TODO in tests/test_save_result_parquet.py where the delayed-vector path currently ends up with shape (2, 1) instead of preserving the attributes like the direct gpd.read_file(...) path does.

EmileSonneveld
EmileSonneveld commented Jun 29, 2026
View reviewed changes
Comment thread openeo_driver/save_result.py
else:
data = data.to_driver_vector_cube()
elif isinstance(data, DriverDataCube):
if isinstance(data, DriverDataCube):

@EmileSonneveld EmileSonneveld Jun 29, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JeroenVerstraelen I found this while checking with mypy. This makes sure the 'data' used in the object that is returned

soxofaan
soxofaan reviewed Jul 3, 2026
View reviewed changes
Comment thread openeo_driver/save_result.py
import xarray

from openeo.metadata import CollectionMetadata
from typeguard import typechecked

@soxofaan soxofaan Jul 3, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this import of typeguard intended, or just temporary during development?

because typeguard was only added as test-dependency in setup.py,
while importing it here in save_result makes it a runtime dependency

@EmileSonneveld EmileSonneveld Jul 3, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this does not work handily. I don't know if it has a performance cost yet, so it would be risky to run it in production directly. I'll close this PR for the moment and only check typing when running in tests.

@EmileSonneveld EmileSonneveld marked this pull request as draft July 3, 2026 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@soxofaan soxofaan soxofaan left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@EmileSonneveld @soxofaan